Skip to content

Conversation

@nicholaspai
Copy link
Member

@nicholaspai nicholaspai commented Mar 21, 2022

I noticed that executeRootProposal was reverting when attempting to send tokens via relayTokens. This PR adds the following fixes to get this to work:

  • bytes data must be passed as the last param to outboundTransfer and the data must include maxSubmissionCost. Specific implementation details are in PR
  • The total ETH to cover maxSubmissionCost + gasPriceBid * gasLimit must be sent as msg.value in outboundTransfer call
  • The specific token's gateway, not router, must be approved to pull the tokens-to-be-bridged into escrow so the HubPool needs to set an allowance

nicholaspai and others added 9 commits March 18, 2022 09:22
* fix[N04] Add additional documentation

Signed-off-by: chrismaree <christopher.maree@gmail.com>

* nit

Signed-off-by: chrismaree <christopher.maree@gmail.com>

* fix[N06] Fix missleading comments

Signed-off-by: chrismaree <christopher.maree@gmail.com>

* Apply suggestions from code review

Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com>

* nit

Signed-off-by: chrismaree <christopher.maree@gmail.com>

Co-authored-by: nicholaspai <9457025+nicholaspai@users.noreply.github.com>
* fix[N02} Move all structs to the same place

Signed-off-by: chrismaree <christopher.maree@gmail.com>

* fix[N03] Fixed inconsistant token metadata versioning

Signed-off-by: chrismaree <christopher.maree@gmail.com>

* nit

Signed-off-by: chrismaree <christopher.maree@gmail.com>

* nit

Signed-off-by: chrismaree <christopher.maree@gmail.com>

* fix[N08] Propose fixes to some naming issues

Signed-off-by: chrismaree <christopher.maree@gmail.com>
Copy link
Contributor

@mrice32 mrice32 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants